Skip to content

fix(cli): smoother streaming table rendering#6345

Merged
wenshao merged 13 commits into
QwenLM:mainfrom
MikeWang0316tw:fix/streaming-table-jitter
Jul 7, 2026
Merged

fix(cli): smoother streaming table rendering#6345
wenshao merged 13 commits into
QwenLM:mainfrom
MikeWang0316tw:fix/streaming-table-jitter

Conversation

@MikeWang0316tw

@MikeWang0316tw MikeWang0316tw commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Follow-up polish to how a live (streaming) markdown table renders in the non-VP TUI, so it streams predictably instead of jittering, flashing, or briefly hanging. All changes are display-only.

  • Atomic rows. A multi-column row passes through intermediate states that are themselves valid rows with fewer cells (| a |, | a | b | on the way to | a | b | c |), so it used to fill in cell by cell. A frontier row is now held back until it has all its columns and appears in one step. The hold-back keys on column count (a table header has ≥2 columns), so a single-pipe line — a shell pipeline | grep foo, a pipe-prefixed log line — still renders and does not vanish.
  • Widths track the current rows. A wider row redraws the whole table once; a narrower row changes nothing (widths are a max over all rows, so they only grow). No per-token reflow.
  • Consistent horizontal-vs-vertical format. The decision to render a table as a horizontal box or the vertical label: value fallback is the same while streaming and once committed, so a table never flips format mid-stream (which reads as a jump).
  • Hold a forming table until it is recognizable. A header (and any partial separator) is held while pending until a separator matching the header's column count arrives, so the header no longer streams in char by char as raw | a | b | text before snapping into a box. Fenced code-block content is left untouched.
  • Defer until the first complete row (no empty-box flip). While a table is still forming, its area shows a clean blank — no raw | a | b | pipe text, no half-filled rows — until the first row completes, at which point the table appears in one step already in its final horizontal/vertical format. This deliberately avoids drawing an empty header box that would then have to flip to the vertical label: value layout once a long first row lands (the format can't be known from the header alone). A clean blank reads better than that flip.

Why it's needed

Building on #6170 (incremental scrollback commit) and #6340 (drop the redundant cue + hold back partial rows), a streaming table still had visible rough edges: cells trickling in one at a time, the whole table re-widening on a wider cell, and the header streaming in as raw pipe text. Each is a display-side artifact; none changes the model output, the markdown, or the table's data.

Reviewer Test Plan

How to verify

Ask the model for a long multi-column markdown table (~60 rows) and watch the live stream at two terminal sizes (e.g. 80×20 and a large window):

  • values no longer fill in cell by cell — each row appears whole once its last column arrives;
  • column widths track content: a wider row redraws the table once, a narrower row leaves it untouched;
  • the header does not stream in char by char as raw | … | text — the box appears once the separator lands;
  • a non-table pipe line (| grep foo) still renders (it is not held back);
  • while the first row streams the area shows a clean blank (no raw pipe text, no half-filled rows), then the table appears whole once the first row completes — it does not draw an empty header box that later flips format;
  • a table renders the same format (horizontal or the vertical label: value list) while streaming and once committed — it does not flip;
  • scrolling up mid-stream does not jump to the top and lock (the fix(cli): stream long responses into scrollback to stop scroll-to-top lock #6170 guarantee still holds).

Unit tests: npx vitest run packages/cli/src/ui/utils/TableRenderer.test.tsx packages/cli/src/ui/utils/MarkdownDisplay.test.tsx → all pass.

Evidence (Before & After)

  • Before: a streaming table shows cells filling in one at a time; a wider row re-widens and repaints the whole table; and the header appears as raw | a | b | text streaming in.
  • After: rows appear whole one at a time; widths only grow; the header appears together with its box once the separator lands; while the first row streams the area shows a clean blank (no raw pipe text, no half-filled rows) rather than an empty box, then the table appears whole; and the horizontal/vertical format is stable between streaming and committed.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Local npm run dev on macOS, streamed against a live model.

Risk & Scope

  • Main risk or tradeoff: display-only, gated on isPending where relevant. The rendered-height clip / safety-net is unchanged, so there is no scroll-to-top regression.
  • Known limitation (documented in code): because column widths track content (redraw-on-wider), a table with very long cell text whose wrapped height sits right at the vertical-fallback threshold can still oscillate format while streaming. Only extreme wide/long-text tables hit it; normal tables stay well clear. Eliminating it would need a content-independent format decision (which mis-sizes common key/value tables) or frozen column widths (losing redraw-on-wider) — not worth it for the residual.
  • Not addressed here: a table whose separator is invalid markdown (e.g. non-ASCII dashes) is still — correctly — rendered as raw text. The rendered-height estimate for tables with wrapping cells (a separate scroll-to-top edge) is handled in a follow-up.
  • Not validated / out of scope: Windows and Linux rendering (macOS only locally; CI covers the rest).
  • Breaking changes: none.

Linked Issues

Relates to #6170 (incremental scrollback commit) and #6340 (cue removal + partial-row hold-back this builds on). No closing keyword.

中文说明

这个 PR 做了什么

延续对 non-VP TUI 中串流中 markdown 表格渲染的打磨,让它串流时更稳定,不再抖动、闪烁或短暂空白。全部是显示层改动。

  • 整列原子化:多栏的一列在打字途中会经过「栏数较少但本身合法」的中间态,原本逐格填入;现在整列凑满所有栏才一次出现。以「栏数 ≥2」判别是不是表头,所以单一 pipe 行(shell 管线 | grep foo、pipe 开头日志)仍会显示、不会消失。
  • 栏宽随当前列:更宽的列整张重画一次;较窄的列不变(栏宽只增不减)。无逐 token reflow。
  • 横式/直式格式一致:横式方框还是直式 label: value 清单的判定,在串流与提交后一致,表格不会中途翻转(翻转看起来像跳)。
  • 成形中的表格先不画:串流中表头(及部分分隔线)会被压住,直到出现「栏数相符的分隔线」才成表格——表头不再以 | a | b | 纯文字逐字冒出。Fenced code block 内容不受影响。
  • 成形中先留干净空白,等第一列到齐才出现:表格还在成形时,该区域保持干净空白(没有纯 | a | b | pipe 文字、也没有半填的列),直到第一列凑满,表格才一步出现、且已经是最终的横式/直式格式。这是刻意不画空表头框——否则一旦长首列到达就得翻成直式 label: value(格式无法只从表头得知),干净空白比那个翻转好读。

为什么需要

#6170#6340 的基础上,串流表格仍有可见粗糙处:储存格逐格冒、更宽储存格让整张重画、表头以纯文字逐字冒。这些都是显示层现象,不改变模型输出、markdown 或表格资料。

Reviewer 测试计划

请模型产生长的多栏 markdown 表格(约 60 列),在两种视窗大小观察串流:数值不再逐格冒、栏宽随内容(更宽重画一次、较窄不动)、表头不再纯文字逐字冒、| grep foo 类非表格文字仍显示、第一列串流时该区域是干净空白(不画空表头框、不会随后翻转格式)、同一张表串流与提交格式一致(不翻转)、往上滚不跳顶锁死。单元测试:npx vitest run packages/cli/src/ui/utils/TableRenderer.test.tsx packages/cli/src/ui/utils/MarkdownDisplay.test.tsx 全通过。

风险与范围

  • 主要风险:仅显示层。算绘高度裁切/安全网不变,无 scroll-to-top 回归。
  • 已知限制(程式码内已注明):因栏宽随内容(redraw-on-wider),内容极长、换行高度刚好卡在直式门槛的表格,串流中仍可能在直/横间抖动。只有极端宽/长文字表格会踩到;一般表格离门槛很远。要根治需「与内容无关的格式判定」(会误判常见 key/value 表)或「冻结栏宽」(失去 redraw-on-wider),不值得。
  • 未处理:分隔线是无效 markdown(如非 ASCII 破折号)的表格仍正确渲染为纯文字;含换行储存格的表格「算绘高度估算」(另一个 scroll-to-top 边界)在后续 PR 处理。
  • 未验证:Windows 与 Linux(本机仅 macOS,CI 覆盖)。
  • 破坏性变更:无。

关联 Issue

关联 #6170#6340。未使用关闭关键字。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: real, observed UX issue. Streaming markdown tables in the interactive TUI exhibit five distinct visual artifacts — cell-by-cell fill-in, width oscillation, vertical-list-then-table flip, raw header text streaming, and a blank hang window before the first row. These are well-documented in prior PRs (#6170, #6340) and represent genuine user-facing polish gaps during live streaming.

Direction: aligned. This is incremental rendering polish for the interactive TUI, a core part of the user experience. Claude Code's CHANGELOG shows a long history of similar streaming/rendering fixes (scroll position jumping, layout jitter, streaming performance improvements), so this area is well within scope.

Approach: five focused refinements across four files, all in packages/cli/src/ui/utils/. Every change is gated on isPending/isStreaming so it only touches the live streaming frontier table — completed tables are unaffected. 308 additions / 32 deletions is reasonable for the scope. No unrelated changes or scope creep.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:真实的、已观察到的 UX 问题。串流中的 markdown 表格在交互式 TUI 中有五种明显的视觉瑕疵——逐格填入、宽度抖动、直式清单再翻横式、表头以纯文字逐字冒出,以及第一列完成前的空白卡住。这些问题在之前的 PR(#6170#6340)中已有记录,确实是 live 串流过程中的用户体验问题。

方向:对齐。这是交互式 TUI 的渲染打磨,属于核心用户体验的一部分。Claude Code 的 CHANGELOG 中有大量类似的串流/渲染修复(滚动位置跳动、布局抖动、串流性能优化),这个方向在范围内。

方案:四个文件中的五项聚焦改进,全部在 packages/cli/src/ui/utils/ 中。所有改动都 gated 在 isPending/isStreaming 上,只影响 live 串流前缘表格——已完成的表格不受影响。308 行增加 / 32 行删除对范围来说是合理的。没有无关改动或范围蔓延。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Reviewed the diff against my independent proposal. The PR's approach matches well — I'd have taken the same general strategy (hold back incomplete table state during streaming, bias toward horizontal format, show empty box early). The implementation is clean and well-scoped.

What I looked at:

  1. Pre-loop header hold-back (MarkdownDisplay.tsx ~L194–227): Walks backwards from the end to find the start of a pipe-line run, checks code fence state, verifies header has a matching separator before keeping the lines. Correct — the code fence guard prevents false positives on |-leading code block content.

  2. Per-row hold-back (MarkdownDisplay.tsx ~L383–403): Enhanced from the previous version to also catch closed-but-incomplete rows (| a | on a 3-column table). The !tableRowMatch || splitMarkdownTableRow(tableRowMatch[1]).length < tableHeaders.length condition correctly covers both unterminated and short-column intermediate states.

  3. Horizontal bias (TableRenderer.tsx ~L597–601): !isStreaming && maxRowLines > MAX_ROW_LINES — clean, minimal change that only affects the live frontier.

  4. Empty table box (MarkdownDisplay.tsx ~L625, TableRenderer.tsx ~L737–758): Removed tableRows.length > 0 guard for the frontier table, skip middle divider when no data rows. Well-commented rationale.

  5. isStreamingFrontier prop flow: Correctly chains through RenderTableRenderTableInternalTableRenderer as isStreaming = isPending && isStreamingFrontier. Only the live frontier table gets isStreamingFrontier={true} — completed tables in the same pending message use the normal path.

No critical bugs, no security concerns, no AGENTS.md violations. Code is focused — every change serves the stated goal.

Unit Tests

 ✓  @qwen-code/qwen-code  src/ui/utils/TableRenderer.test.tsx (67 tests) 267ms
 ✓  @qwen-code/qwen-code  src/ui/utils/MarkdownDisplay.test.tsx (144 tests) 642ms

 Test Files  2 passed (2)
      Tests  211 passed (211)

All 211 tests pass. New tests cover all five refinements: atomic rows, width tracking, horizontal bias, header hold-back, empty box rendering, and code block safety.

Tmux Real-Scenario Test

Both the installed build (qwen -p) and the PR code (npm run dev -- -p) were run in tmux with the same prompt asking for a 10-row, 4-column table.

Before (installed build, v0.19.1)

| Capital | Country | Population | Timezone |
|---|---|---|---|
| Tokyo | Japan | 13.96M | UTC+9 |
| Berlin | Germany | 3.75M | UTC+1 |
| Brasília | Brazil | 3.04M | UTC-3 |
| Ottawa | Canada | 1.02M | UTC-5 |
| Canberra | Australia | 457K | UTC+10 |
| Cairo | Egypt | 10.1M | UTC+2 |
| New Delhi | India | 329K | UTC+5:30 |
| Buenos Aires | Argentina | 3.08M | UTC-3 |
| Nairobi | Kenya | 4.73M | UTC+3 |
| Oslo | Norway | 709K | UTC+1 |

After (this PR via dev build)

| Capital | Country | Population | Timezone |
|---------|---------|------------|----------|
| Tokyo | Japan | 13,960,000 | UTC+9 |
| Paris | France | 2,161,000 | UTC+1 |
| Brasília | Brazil | 3,055,000 | UTC-3 |
| Nairobi | Kenya | 4,397,000 | UTC+3 |
| Canberra | Australia | 466,000 | UTC+10 |
| Ottawa | Canada | 1,017,000 | UTC-5 |
| Berlin | Germany | 3,645,000 | UTC+1 |
| New Delhi | India | 321,000 | UTC+5:30 |
| Cairo | Egypt | 10,100,000 | UTC+2 |
| Mexico City | Mexico | 9,209,000 | UTC-6 |

Note: Print mode (-p) outputs raw markdown — the TUI box-drawing rendering (the actual fix target) only appears in interactive mode. Both builds produce correct table data. The visual refinements (hold-back, horizontal bias, empty box, atomic rows) can only be verified in the interactive Ink-based TUI during live streaming, which requires an interactive model session. The unit tests comprehensively cover all five behaviors with 211 passing assertions.

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Reflection

This PR is a clean, well-scoped follow-up to #6170 and #6340 — each of the five refinements addresses a specific, documented streaming-table artifact. The implementation matches what I'd have done independently: hold back incomplete state at the frontier, bias toward horizontal format, show the empty box early to avoid the blank-hang illusion.

The code is straightforward. The isStreamingFrontier prop is a clean abstraction that correctly isolates the live frontier table from completed tables in the same pending message. The pre-loop header hold-back is the most intricate piece, but it's well-guarded (code fence check, column count match) and handles the edge cases.

211 unit tests pass, covering all five behaviors plus the code-block safety net. The tmux test in -p mode confirms both builds produce correct table data; the interactive TUI visual rendering (the actual fix target) isn't capturable in -p mode, but the unit tests exercise the rendering logic thoroughly.

No scope creep, no unrelated changes, no over-abstraction. Every line in the diff serves the stated goal. The comments explain why without being verbose. This is the kind of focused polish PR that's easy to review, revert if needed, and maintain.

Approved. ✅

中文说明

反思

这个 PR 是 #6170#6340 的干净、聚焦的后续——五项改进中的每一项都针对一个特定的、有记录的串流表格视觉瑕疵。实现方式与我的独立方案一致:在串流前缘压住不完整状态、偏向横式格式、提前显示空表框以避免空白卡住的错觉。

代码简洁明了。isStreamingFrontier prop 是一个干净的抽象,正确地将 live 前缘表格与同一 pending 消息中的已完成表格隔离开来。循环前的表头压住是最复杂的部分,但它有良好的防护(code fence 检查、栏数匹配)并处理了边缘情况。

211 个单元测试全部通过,涵盖了所有五种行为以及 code block 安全网。tmux 测试在 -p 模式下确认两个版本都能产生正确的表格数据;交互式 TUI 视觉渲染(实际的修复目标)在 -p 模式下无法捕获,但单元测试充分测试了渲染逻辑。

没有范围蔓延、没有无关改动、没有过度抽象。diff 中的每一行都服务于既定目标。注释解释了 为什么 而不冗长。这是一种聚焦的打磨 PR——易于审查、必要时易于回滚、易于维护。

已批准 ✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

Comment thread packages/cli/src/ui/utils/TableRenderer.tsx
Follow-up to the streaming table hold-back, on its own branch so the
cue-removal PR (QwenLM#6340) can land undisturbed. Makes a live table stream
predictably instead of jittering, flashing, or hanging.

- Atomic rows: hold a frontier row back until it has ALL its columns. A
  multi-column row passes through intermediate states that are themselves
  valid rows with fewer cells (`| a |`, `| a | b |` toward `| a | b | c |`),
  so the old hold-back let it fill in cell by cell. Now the whole row
  (border + every cell) appears in one step.

- Widths track the current rows (no freeze): a wider row redraws the whole
  table once; a narrower row changes nothing (widths are a max over all
  rows, so they only ever grow). Redraw-on-wider only, never per token.

- Bias the streaming preview to the horizontal format: while a table is the
  live frontier it only falls back to the vertical `label: value` list when
  the terminal is genuinely too narrow, not because an early row wraps tall.
  This stops a table from briefly rendering as a vertical list and then
  flipping to a horizontal table (a visible jump).

- Hold a forming table back until it is recognizable: a header (and any
  partial separator) is trimmed while pending until a separator matching the
  header's column count arrives, so the header no longer streams in char by
  char as raw `| a | b |` text before snapping into a box. Fenced code-block
  content is left untouched.

- Draw the empty header box as soon as the table is recognized, before the
  first row completes, so the table area does not sit blank (no box, no cue)
  and look like a hang if generation stalls in that window. A zero-row box
  omits the header/body divider so it reads as a clean header, not an empty
  row.

Only the live frontier table is affected; completed and committed tables use
the normal logic. 211 tests pass (MarkdownDisplay + TableRenderer).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。

@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Good catch — fixed in 53db9ccad.

Zero-row header box on a narrow terminal (TableRenderer.tsx): the width trigger (contentWidth < minHorizontalTableWidth) forced the vertical format, and renderVerticalFormat() over empty rowMetrics yields an empty string — a blank box, exactly the hang the empty-box was meant to avoid. Applied your suggestion so a zero-row table stays horizontal:

const useVerticalFormat =
  (rowMetrics.length > 0 && contentWidth < minHorizontalTableWidth) ||
  (!isStreaming && maxRowLines > MAX_ROW_LINES);

Added a regression test (keeps the zero-row header box horizontal on a narrow terminal, contentWidth 20) asserting the header + box render instead of blank. 212 tests pass, lint clean.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] The downstream safety check at TableRenderer.tsx:767 (maxLineWidth > contentWidth - SAFETY_MARGIN) is a second path to renderVerticalFormat() that is NOT guarded for zero-row tables — the rowMetrics.length > 0 guard at line 604 only protects useVerticalFormat. When the safety fires on a zero-row streaming table (6+ columns on a ~25-col terminal), renderVerticalFormat() iterates empty rowMetrics and returns ''. The header box the PR aims to draw immediately vanishes entirely.

This is the same root cause as the useVerticalFormat guard at line 604 (raised in the prior review thread there), but the fix only covered one of the two fallback paths. The safety check needs the same rowMetrics.length > 0 guard:

if (maxLineWidth > contentWidth - SAFETY_MARGIN && rowMetrics.length > 0) {

Comment thread packages/cli/src/ui/utils/MarkdownDisplay.tsx
Review follow-up (two [Critical] findings).

- TableRenderer: the maxLineWidth safety check is a second path to the
  vertical format, unguarded for zero-row tables. On a very narrow terminal
  a zero-row streaming header box would fall through it and render an empty
  string — the box vanishes. Skip that fallback when there are no rows so the
  header stays visible even if it slightly overflows.

- MarkdownDisplay: the pre-loop header hold-back trimmed ANY trailing run of
  pipe-leading lines. When the first line is not a complete `| … |` row,
  headerCells was 0 and the run was trimmed anyway — so non-table pipe text
  (an un-fenced shell pipeline `| grep foo`, pipe-prefixed log output) would
  vanish from the live preview until commit. Only hold back when the first
  pipe-line is a plausible table header (a complete row).

Tests cover both. 215 pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Both [Critical] findings fixed in 431fe2782 (pushed as a new commit, no force-push this time — thanks for the reminder).

  1. TableRenderer safety-check path (maxLineWidth > contentWidth - SAFETY_MARGIN): guarded with rowMetrics.length > 0, so a zero-row streaming header box no longer falls through to renderVerticalFormat() (empty string → blank). It stays horizontal even if it slightly overflows a very narrow terminal — visible beats vanished. Test: 6-column zero-row box at contentWidth 25 still renders the box.

  2. Pre-loop header hold-back capturing non-table text: now only trims when the first pipe-line is a plausible table header (a complete | … | row, per your suggestion). Un-fenced shell pipelines (| grep foo) and pipe-prefixed log output no longer vanish from the live preview. Test: | grep foo renders as text while pending.

215 tests pass, lint clean.

The previous commit (restricting the header hold-back to a complete `| … |`
row, to stop non-table pipe text from vanishing) reintroduced the cell-by-
cell header flash: while a header is typed (`| Alpha`, `| Alpha | Bet`, …)
it is not yet a complete row, so it rendered as raw text.

Discriminate by column count instead of closed-ness: a table header has ≥2
columns; a single-pipe line (shell pipeline `| grep foo`, pipe-prefixed log)
has one cell. Count cells on the first line whether or not it is closed, and
hold the run only when it has ≥2 columns and no matching separator yet. So a
multi-column header held mid-type no longer flashes, while single-pipe non-
table text still renders (the earlier [Critical] fix stands). A header still
typing its very first cell is indistinguishable from a single-pipe line, so
it shows briefly until the second column appears — the narrowest flash
possible without hiding real pipe text.

217 tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Heads-up / self-correction: my previous commit 431fe2782 (restricting the header hold-back to a complete | … | row per the [Critical] non-table-text fix) reintroduced the cell-by-cell header flash — while a header is typed (| Alpha, | Alpha | Bet, …) it is not yet a complete row, so it rendered as raw text.

Fixed in 77c5320b8 by discriminating on column count instead of closed-ness: a table header has ≥2 columns; a single-pipe line (| grep foo, pipe-prefixed log) has one cell. The run is held only when its first line has ≥2 cells (closed or not) and no matching separator yet. Net result:

  • multi-column header held mid-type → no flash;
  • single-pipe non-table text still renders → the earlier [Critical] fix stands (test unchanged).

One residual by necessity: a header still typing its first cell is indistinguishable from a single-pipe line, so it shows briefly until the second column appears — the narrowest flash possible without hiding real pipe text. Added a test for the incomplete multi-column header. 217 tests pass, lint clean.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI still running.

No high-confidence issues found. Three low-confidence observations for human review:

  1. Multi-pipe shell commands (| grep error | sort | uniq) outside code fences are held back during streaming because they produce ≥2 non-empty cells, passing the table-header heuristic. Impact is narrow — multi-pipe commands in LLM output are almost always inside fenced code blocks, and content reappears when streaming ends. (MarkdownDisplay.tsx:221-233)

  2. Pre-loop and main-loop column counting use different methods (.filter(c => c.length > 0) on full line vs. unfiltered regex capture group). They disagree for headers with empty cells like | A | B | |. (MarkdownDisplay.tsx:220 vs :366)

  3. End-of-content handler removed the tableRows.length > 0 guard too broadly — a committed zero-row table at end-of-content now renders an empty header box, while the same table mid-message still requires rows. Suggested: if (inTable && tableHeaders.length > 0 && (tableRows.length > 0 || isPending)). (MarkdownDisplay.tsx:636)

Build passes. 217 tests pass (148 MarkdownDisplay + 69 TableRenderer). ESLint clean.

— qwen3.7-max via Qwen Code /review

MikeWang0316tw and others added 2 commits July 6, 2026 07:26
The horizontal-vs-vertical bias (force a live table horizontal while
streaming) backfired for tables that genuinely belong in the vertical
`label: value` format — a wide table with many columns of long, wrapping
text. It rendered horizontal (tall, clamped, looking stuck) while it was
the streaming frontier, then flipped to vertical the moment it stopped
being the frontier (the next block started) or committed — a visible
format flip, and worse than the vertical-list flash it was meant to avoid.

Drop the streaming bias: the horizontal-vs-vertical decision is now the
same while pending and once committed, so a table never flips format
between the two. Removes the now-unused isStreaming / isStreamingFrontier
plumbing.

Known residual (pre-existing, not from this change): because column widths
track content (redraw-on-wider), a borderline table's wrapped-row height
can still cross the vertical threshold mid-stream. Fully stabilizing that
needs a content-independent format decision — a separate change.

217 tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Document the accepted limitation next to the horizontal-vs-vertical
decision: because column widths track content (redraw-on-wider), a table
with very long cell text sitting right at MAX_ROW_LINES can still oscillate
format while streaming. Only extreme wide/long-text tables hit it; the
alternatives (content-independent decision, or frozen widths) each cost
more than the residual is worth.

Comment-only; no behaviour change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Update since the earlier review (description refreshed to match):

  • Dropped the "horizontal bias" approach (force a live table horizontal while streaming). It backfired for tables that genuinely belong in the vertical label: value format (many columns of long wrapping text): they rendered horizontal while at the frontier, then flipped to vertical once the next block started or on commit — a visible format flip. The format decision is now the same while streaming and once committed, so a table never flips format mid-stream. Removed the now-unused isStreaming / isStreamingFrontier plumbing.
  • Also folded in the earlier fixes: multi-column header held mid-type without hiding non-table pipe text; the two zero-row / non-table [Critical] guards.
  • Documented one accepted limitation in code: with redraw-on-wider column widths, an extreme long-text table sitting right at the vertical threshold can still oscillate format while streaming (normal tables stay clear).

All commits will squash on merge. npx vitest run packages/cli/src/ui/utils/TableRenderer.test.tsx packages/cli/src/ui/utils/MarkdownDisplay.test.tsx → 217 pass.

Comment thread packages/cli/src/ui/utils/MarkdownDisplay.tsx Outdated
The streaming hold-back counted header columns on the full line with
empty cells filtered out, while the main table detector strips the outer
pipes and splits without filtering. For a header with an empty-named
column like `| A || B |` the two disagreed (2 vs 3), so the hold-back
never found the matching 3-column separator and hid the table for the
whole stream. Count columns the same way in both places.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread packages/cli/src/ui/utils/MarkdownDisplay.tsx
The streaming hold-back keeps a run of pipe-lines back until a matching
separator arrives, so a real multi-column header does not flash in cell
by cell. But multi-cell non-table pipe content — a shell pipeline
(`| grep foo | wc -l`), a log excerpt (`| 200 | OK | GET /x`), an
ASCII-art border — also has >=2 cells, so it was held for the entire
stream and only appeared on commit.

A markdown table's separator is the line immediately after the header, so
once a line follows the header and does not even begin like a separator
(optional pipe, optional colon, then a dash), the run is decided: not a
forming table. Release it. A lone header still being typed (no line after
it yet) is still held, so the no-flash behavior is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Addressed the [Suggestion] on multi-cell non-table pipe content (commit — normal push, no force-push).

Rather than a line-count limit, I keyed on the markdown rule that a table's separator is the line immediately after the header. So once a line follows the held header and doesn't even begin like a separator (optional pipe, optional colon, then a dash), the run is decided as not-a-table and released:

  • shell pipelines (| grep foo | wc -l), log excerpts (| 200 | OK | GET /x) and ASCII-art borders now render live instead of being hidden until commit;
  • a lone header still being typed (no line after it yet) is still held, so the no-cell-by-cell-flash behavior is unchanged;
  • a header whose separator is still being typed (|--) also stays held (it does begin like a separator).

Added a test for the released log-excerpt case. 221 tests pass (152 MarkdownDisplay + 69 TableRenderer), lint clean.

The horizontal-vs-vertical choice used maxRowLines measured over EVERY row,
so a table that started horizontal (short first row) flipped to vertical the
moment a later, taller-wrapping row streamed in — a visible mid-stream format
change. Measure only the header + the first data row instead. The first row is
representative for the common case, so the format is decided once and stays
put as rows append. Column widths still track all rows (redraw-on-wider is
unchanged); only the format choice is anchored.

Trade-off: a table whose first row is short but a later row wraps very tall
stays a (taller) horizontal grid rather than flipping to vertical — rare, and
preferable to a visible flip.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Follow-up (commit — normal push, no force-push): the horizontal-vs-vertical format choice no longer flips mid-stream.

Previously maxRowLines was measured over every row, so a table that started horizontal (short first row) flipped to vertical the moment a later, taller-wrapping row streamed in — a visible format change on a long-text table. It now measures only the header + first data row, so the format is decided once and stays put as rows append. Column widths still track all rows (redraw-on-wider is unchanged); only the format choice is anchored.

Trade-off: a table whose first row is short but a later row wraps very tall stays a (taller) horizontal grid rather than flipping — rare, and preferable to a visible flip. This tightens the earlier "format decision consistent, not streaming-biased" commit (it was consistent for a given row set, but the set grew during streaming).

Added a TableRenderer test (short first row + tall later row stays horizontal). 222 tests pass (70 TableRenderer + 152 MarkdownDisplay), lint clean.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No high-confidence issues found. Downgraded from Approve to Comment: CI still running.

Summary: Well-scoped display-only improvements to streaming table rendering. 222 tests pass including 20 new test cases. The changes are thoughtfully implemented with honest documentation of accepted trade-offs.

Low-confidence observations for human review:

  1. couldStillBeTable regex (MarkdownDisplay.tsx:251) is slightly too permissive — it matches data rows whose first cell starts with a dash (e.g. | --verbose | Enable verbose output | after a multi-column header). Such a table would be held back for the entire streaming session. Consider tightening with tableSeparatorRegex.test(lineAfterHeader).

  2. calculateMaxRowLines (TableRenderer.tsx:556) measures the first row against columnWidths computed from all rows, so the vertical-format decision can still flip when a later wider row forces column redistribution. Acknowledged as an accepted limitation in the PR description.

— qwen3.7-max via Qwen Code /review

MikeWang0316tw and others added 2 commits July 6, 2026 13:02
The "could this pipe run still become a table?" check treated any line after
the header that merely started with a dash as a possible separator, so an
options table whose first data cell begins with a flag — `| --verbose | … |`
— was held back for the whole stream. Use tableSeparatorRegex instead: it
still matches a partial separator being typed (`|--`) so a real header is
held until its separator lands, but rejects a dash-led data cell (trailing
letters), which now renders live.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lip)

A recognized table with no complete data row yet was drawn immediately as an
empty header box. A zero-row table can only render horizontally (the vertical
fallback needs rows), so once a long first row landed the box flipped to the
vertical label:value format — a visible format change that cannot be avoided
by looking at the header alone (column names are short; width comes from the
values). Defer the table while pending until its first row completes, so it
first appears already in its final format with no flip.

Cost: the table area stays blank while the header + first row stream (the
pre-loop trim already hid the header text, so this only extends that blank).
Committed tables always have rows, so their behavior is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Two follow-up commits (normal push, no force-push):

1. Release a dash-led data row from the hold-back — addresses observation ① from the last review. The couldStillBeTable check treated any post-header line starting with a dash as a possible separator, so an options table whose first data cell is a flag — | --verbose | … | — was held back for the whole stream. Now uses tableSeparatorRegex.test(lineAfterHeader): a partial separator being typed (|--) is still held, but a dash-led data cell (trailing letters) renders live. Added a test.

2. Defer a streaming table until its first row (no empty-box flip) — a recognized table with no complete data row yet was drawn immediately as an empty header box. A zero-row table can only render horizontally (the vertical fallback needs rows to lay out), so once a long first row landed the box flipped to the vertical label: value format — a visible format change. The format genuinely can't be decided from the header alone (column names are short; the width that forces vertical comes from the values). So while pending, defer the table until its first row completes: it first appears already in its final format, no flip. Cost: the table area stays blank while the header + first row stream (the pre-loop trim already hid the header text, so this only extends that blank). Committed tables are unchanged. Updated the empty-box test to assert the deferral.

Together with the earlier first-row-anchored format decision, a streaming table no longer flips horizontal↔vertical at any point. Tests: 156 MarkdownDisplay + 70 TableRenderer pass, lint clean.

Comment thread packages/cli/src/ui/utils/MarkdownDisplay.tsx Outdated
Comment thread packages/cli/src/ui/utils/MarkdownDisplay.tsx Outdated
Comment thread packages/cli/src/ui/utils/MarkdownDisplay.tsx Outdated
Comment thread packages/cli/src/ui/utils/TableRenderer.tsx
Comment thread packages/cli/src/ui/utils/TableRenderer.test.tsx Outdated
… committed format

Five review findings:

- [Critical] The pre-loop hold-back's code-fence check used a naive boolean
  toggle that ignored fence char/length, so a nested fence (```` with an inner
  ```) mis-closed and a real code line like `| A | B |` was held back and
  vanished while streaming. Track the open fence's delimiter and validate the
  close (same char, >= length), mirroring the main parser.
- A COMPLETE separator whose column count already differs from the header can
  never match, so release the pipe run instead of holding it for the whole
  stream (the main parser treats it as text).
- The end-of-content table flush now uses the same `tableRows.length > 0` guard
  as the mid-content handler, so a degenerate zero-row table behaves the same
  whichever way it ends — no EOF-vs-mid asymmetry.
- TableRenderer's first-row-only maxRowLines (no-flip) applied to committed
  tables too; a committed short-first-row + tall-later-row table wrongly stayed
  horizontal. Gate on a new `isPending` prop: measure the first row only while
  streaming, every row once committed (most readable, no flip concern).
- Renamed the test block that claimed a nonexistent `isStreaming` prop; added
  committed-vs-streaming format tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@MikeWang0316tw

MikeWang0316tw commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all five findings from the last review (one commit, normal push):

  • [Critical] code-fence tracking (MarkdownDisplay.tsx): the pre-loop hold-back used a naive boolean toggle. It now tracks the open fence's delimiter and validates the close (same char, length ≥ open), mirroring the main parser — a nested ```` … ``` no longer mis-closes and hides a code | A | B | line while streaming. Added a nested-fence test.
  • Mismatched complete separator: a separator that is already complete (ends with |) but whose column count differs from the header can never match, so it is released instead of held for the whole stream (the main parser treats it as text). Added a test.
  • Zero-row EOF/mid asymmetry: the end-of-content flush now uses the same tableRows.length > 0 guard as the mid-content handler, so a degenerate zero-row table behaves identically either way.
  • First-row measurement leaked to committed tables: calculateMaxRowLines measured only the first row unconditionally, so a committed short-first-row + tall-later-row table wrongly stayed horizontal. Added an isPending prop to TableRenderer — measure the first row only while streaming (no mid-stream flip), every row once committed (most readable). Added committed-vs-streaming tests.
  • Test naming: renamed the block that referenced a nonexistent isStreaming prop.

231 tests pass (160 MarkdownDisplay + 71 TableRenderer), lint clean.

Comment thread packages/cli/src/ui/utils/MarkdownDisplay.tsx Outdated
Comment thread packages/cli/src/ui/utils/MarkdownDisplay.test.tsx
A table closed by a following line is complete even while the message keeps
streaming, but it was still rendered with the first-row-only format anchor —
so a short-first-row + tall-later-row mid-content table showed horizontal and
then flipped to vertical the moment the message committed.

Split the two concerns that were both riding on `isPending`:
- the height clamp still tracks whether the MESSAGE is streaming (so a
  mid-content table stays bounded and the estimator's clamped cost can't
  under-estimate the render);
- the format anchor now tracks whether THIS TABLE is the streaming frontier.
  The mid-content flush passes isFrontier={false} → all rows measured → final
  format now; only the end-of-content (frontier) table anchors to the first row.

Renamed TableRenderer's format-anchor prop to `isStreaming` (it is not the
message-level pending flag). Added mid-content and tilde-fence tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Addressed both suggestions from the last review (one commit, normal push):

  • Completed mid-content table flipping format at commit: a table closed by a following line is complete even while the message keeps streaming, but it was still rendered with the first-row-only format anchor — so a short-first-row + tall-later-row mid-content table showed horizontal and then flipped to vertical at commit. Rather than passing isPending={false} there (which would also drop the height clamp and let a big mid-content table overflow), I split the two concerns: the height clamp still tracks whether the message is streaming (mid-content table stays bounded, so the estimator's clamped cost can't under-estimate the render), while the format anchor now tracks whether this table is the streaming frontier. The mid-content flush passes isFrontier={false} (measure all rows → final format now); only the end-of-content frontier table anchors to the first row. Renamed TableRenderer's format-anchor prop isPendingisStreaming to reflect this.
  • Tilde-fence coverage: added a test that an open ~~~ fence is not closed by an inner ``` (different char), guarding the fence-char check for tilde fences.

235 tests pass (164 MarkdownDisplay + 71 TableRenderer), lint clean.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No high-confidence issues found. Downgraded from Approve to Comment: CI still running.

Summary: Well-scoped display-only improvements to streaming table rendering. 235 tests pass (71 TableRenderer + 164 MarkdownDisplay). Deterministic analysis (tsc, eslint) clean. 9 review agents + reverse audit found 0 high-confidence issues. One low-confidence observation (math-block fence state divergence) flagged for human review.

— qwen3.7-max via Qwen Code /review

The streaming table hold-back tracked code fences so a `| A | B |` code line
would render, but not display-math (`$$ … $$`) blocks. The main parser pushes
math content verbatim (never as a table), so a `| a | b |` norm/matrix line at
the frontier of an open math block was treated as a forming table and blanked
until the block closed. Track math fences in the trim's fence scan too, mirroring
the main parser's precedence (code block wins, then math), and skip the hold-back
while inside one. Addresses the low-confidence review observation on QwenLM#6345.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Addressed the low-confidence observation from the latest review ($$ math-block fence state divergence) in f05d64c.

The streaming hold-back trim tracked code fences but not display-math blocks, while the main parser pushes $$ … $$ content verbatim (never as a table). So a | a | b | norm/matrix line at the frontier of an open math block was treated as a forming table and blanked until the block closed. The trim's fence scan now also tracks math fences, mirroring the main parser's precedence (code block wins, then math), and skips the hold-back while inside one. Added a regression test (does not hold a pipe line inside an open display-math block); it fails without the guard and passes with it. Full suite: 237 tests pass (71 TableRenderer + 166 MarkdownDisplay), tsc/eslint clean on the touched files.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No high-confidence issues found. Downgraded from Approve to Comment: CI still running.

Summary: Well-scoped display-only improvements to streaming table rendering. 237 tests pass (71 TableRenderer + 166 MarkdownDisplay), including ~20 new test cases covering atomic rows, format stability, header hold-back, and zero-row header boxes. Deterministic analysis (tsc + eslint) clean. The implementation is thoughtful, the edge cases are well-handled, and the author has been responsive to prior feedback.

Two low-confidence observations for human review:

  1. When a frontier table closes mid-stream via a following non-table line, isFrontier flips from true to false, causing all rows to be measured — potentially flipping horizontal→vertical in one frame. The code comment acknowledges this trade-off.
  2. couldStillBeTable inspects only rest[0] while hasMatchingSeparator scans all of rest — a narrow edge case where a forming separator that never completes could hold back content.

— qwen3.7-max via Qwen Code /review

@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Thanks — on the two low-confidence observations from the latest review, both are intentional and need no change:

1. isFrontier true→false one-frame format flip when a mid-content table closes. This is the accepted redraw-on-wider trade-off already documented in the code comment (the review notes this). Measuring only the first row while a table is the streaming frontier is what prevents the repeated mid-stream vertical↔horizontal flipping as rows arrive; the cost is that a mid-content table can settle to its all-rows format in a single frame when it stops being the frontier. Eliminating that frame would mean measuring all rows while streaming, which reintroduces the per-row flipping this PR removes. Net trade acknowledged in-code.

2. couldStillBeTable inspects only rest[0] while hasMatchingSeparator scans all of rest. The asymmetry is deliberate and matches GFM semantics: a table's separator must be the line immediately after the header, so "could this still become a table?" is decided solely by rest[0], whereas "is there already a complete matching separator?" scans the run to recognize a fully-formed table. The comment above lineAfterHeader documents this. There is also no permanent hold-back: the trim runs only while isPending, so if a forming separator never completes, the stream ends, isPending flips false, the trim is skipped, and the run renders as raw text — no content loss.

The earlier math-block observation is fixed in f05d64c with a regression test. CI is green; 237 tests pass, tsc/eslint clean.

@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Note for maintainers — the CHANGES_REQUESTED label is stale.

All findings from the bot's CHANGES_REQUESTED review (06:55Z) have been addressed:

Finding Fix
[Critical] hold-back code-fence tracking used a naive toggle (no fence char/length validation) Now validates the open fence's delimiter char and length, mirroring the main parser (MarkdownDisplay.tsx)
Mid-content vs end-of-content zero-row table handled inconsistently Both handlers now gate on tableHeaders.length > 0 && tableRows.length > 0 (a zero-row table is deferred, not dropped mid-content / shown at EOF)
A complete separator with a mismatched column count held the run back for the whole stream Released when sepCols !== headerCells
calculateMaxRowLines measured only header + first row for committed tables too Added an isStreaming prop to TableRenderer; committed tables measure all rows, only streaming tables measure the first row
Test describe('(isStreaming)') named a prop that didn't exist Renamed to streaming table rendering (format stability)

Every subsequent bot review (08:43Z, 10:10Z, 11:43Z) reports 0 high-confidence issues; the remaining items are the bot's own low-confidence observations, addressed in the two comments above ($$ math-block hold-back fixed in f05d64c with a regression test; the isFrontier one-frame trade-off and the rest[0] separator-position asymmetry are intentional and documented in-code).

CI is green and 237 tests pass. The CHANGES_REQUESTED state persists only because the bot has posted COMMENTED reviews since (each noting "Downgraded from Approve to Comment: CI still running"), and a COMMENTED review does not clear a prior CHANGES_REQUESTED — only an APPROVED or a dismissal does. Nothing further is outstanding on the code.

@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Hi @wenshao — since this PR directly continues #6340 (which you reviewed and merged), would you mind taking a look when you have a chance? 🙏

It's ready from my side:

  • All bot review findings are addressed (the per-finding mapping is in the comment above).
  • CI is green and 237 tests pass; tsc/eslint clean.
  • The remaining CHANGES_REQUESTED state is stale: the bot filed it earlier, then only posted COMMENTED reviews since (each noting "Downgraded from Approve to Comment: CI still running"), and a COMMENTED review doesn't clear a prior CHANGES_REQUESTED — the latest review reports 0 high-confidence issues.

No rush — thanks for your time!

@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review. Every finding from the review passes has been fixed in follow-up commits (normal pushes — no force-push since the initial squash), and I've now resolved all conversation threads. Summary:

Review finding Fixed in
[Critical] code-fence tracking used a naive boolean toggle (no fence char/length validation) 3c01a3fea, refined in 76727efa4 — now validates fence char + length like the main parser
[Suggestion] committed zero-row table mid-content silently dropped vs. header-box at end-of-content 3c01a3fea — mid-content (:502) and end-of-content (:718) now share the same tableRows.length > 0 guard, so zero-row behaviour is symmetric
[Suggestion] header/separator column-count mismatch held the whole pipe run back for the rest of the stream fd89f8581 + fb06b16bc — mismatched / dash-led rows are released
[Suggestion] calculateMaxRowLines measured only header + first row for committed tables too (no isStreaming prop) 76727efa4 — added isStreaming prop; committed tables (isStreaming=false) measure all rows (TableRenderer.tsx:575)
[Suggestion] test describe named (isStreaming) but no such prop exists 3c01a3fea — renamed
[Suggestion] isPending forwarded unconditionally to mid-content tables addressed (now outdated) via the isFrontier decoupling in 76727efa4
[Suggestion] hold-back fence tracker startsWith prevents backtick/tilde cross-close (test) covered by the fence-char validation added alongside 76727efa4

Unit tests green locally:

npx vitest run packages/cli/src/ui/utils/TableRenderer.test.tsx packages/cli/src/ui/utils/MarkdownDisplay.test.tsx

@qwen-code-ci-bot could I get a re-review to clear the stale CHANGES_REQUESTED? Happy to address anything else. 🙏

@wenshao

wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

✅ Local real-build verification — PR #6345

Built this branch (fix/streaming-table-jitter, HEAD f05d64c9) on top of its merge-base 1b58ede8 (#6340, the PR this builds on) and verified the streaming behavior end-to-end on Linux — which also fills the ⚠️ Linux gap in the Tested-on table. Every headline improvement holds; unit tests + lint + types are green. I found no correctness bugs; there is one documentation nit worth fixing before merge (below).

How I verified

Layer Method Result
Unit (your Reviewer Test Plan) vitest run TableRenderer.test.tsx MarkdownDisplay.test.tsx 237 pass (71 + 166)
Static gates eslint --max-warnings 0 · tsc --noEmit clean
Component (real) Drove the compiled MarkdownDisplay through a simulated token stream (isPending=true) via ink-testing-library, BEFORE (#6340) vs AFTER (#6345)
Full app (real) Fake streaming OpenAI server → real qwen TUI in tmux → captured the table streaming in row-by-row

Real qwen TUI — streaming a 6-row table (this PR)

real qwen TUI streaming a markdown table

  • t ≈ 2.5s — only the sentence shows; the header does not stream in as raw | … | text.
  • t ≈ 6.5s — rows appear whole, one at a time, as a clean box (redraw-on-wider: the Role column widens once when Secondary lands).
  • t ≈ 12s — committed 6-row table, same horizontal format (no flip).

Before (#6340) vs After (#6345) — identical token stream

before vs after streaming frontier

  • Header hold-back ✅ — BEFORE leaks | Name | Role | Team | as raw text; AFTER hides it until the box is ready.
  • Atomic rows ✅ — BEFORE renders | Alice | as a row with empty Role/Team cells (and re-widens as each cell lands); AFTER holds the row until all 3 columns arrive, then it appears in one step.
  • Widths / no per-token reflow ✅ and format stability while streaming ✅ (a tall row arriving mid-stream does not flip horizontal→vertical — it stays anchored to row 1). | grep foo (single pipe) still renders.

One thing worth fixing before merge — description, not code

The description's "No blank hang — … its header box is drawn immediately (before the first row completes), so the area does not sit blank" (and Evidence → After: "a header box is shown immediately so the area never sits blank") no longer match the shipped code. Commit 30f1610a ("defer a streaming table until its first row (no empty-box flip)") added a tableRows.length > 0 guard at both RenderTable emit sites (MarkdownDisplay.tsx:502 and :718), so a zero-row table is never emitted while pending. The real TUI confirms it: at t ≈ 2.5s the table area is blank (just the preamble) until Alice's row completes.

This is arguably the better behavior — a clean blank beats an empty box that then flips to the vertical label: value layout once a long first row lands — so I'd keep the code and reword the bullet, e.g.:

"the area shows a clean blank (no raw pipe text, no half-filled rows) until the first complete row, avoiding an empty-box → vertical flip."

Side note: with that guard, the zero-row header-box path in TableRenderer.tsx — including the safety-check guard added for the earlier [Critical] review at :782 — is currently unreachable from its only caller (MarkdownDisplay). Harmless / defensive, but worth knowing it isn't exercised on this path.

Minor notes (no action needed)

  1. "format stable between streaming and committed" holds during streaming and for uniform tables (verified: streaming format == committed format). A pathological short-first-row / tall-later-row table still flips horizontal→vertical once at commit (streaming anchors to row 1 → horizontal; committed measures all rows → vertical). The calculateMaxRowLines comment already documents this — just noting the Evidence wording is slightly stronger than the behavior.
  2. A multi-pipe non-table line (| grep foo | wc -l, ≥2 cells) is held back while streaming until a following non-separator line releases it — reproduced. Accepted tradeoff (such commands are almost always fenced); single-pipe | grep foo renders immediately.

Verdict: the streaming-table improvements are real and verified in the actual app, and are a clear step up from #6340; tests/lint/types green. Recommend a quick reword of the "no blank hang / immediate header box" bullet to match the shipped "defer until first row" behavior before merge — everything else is documented or minor.

中文说明

✅ 本地真实构建验证 — PR #6345

在其 merge-base 1b58ede8(即本 PR 依赖的 #6340)之上构建了该分支(fix/streaming-table-jitter,HEAD f05d64c9),并在 Linux 上做了端到端验证 —— 顺带补上了 Tested-on 表里 ⚠️ 的 Linux 空缺。所有主要改进都成立;单测 + lint + 类型检查全绿。没有发现正确性 bug;有 一处文档描述与实现不符,建议合并前修正(见下)。

验证方式

层次 方法 结果
单元测试(你的 Reviewer Test Plan) vitest run TableRenderer.test.tsx MarkdownDisplay.test.tsx 237 通过(71 + 166)
静态门禁 eslint --max-warnings 0 · tsc --noEmit 干净
组件级(真实) ink-testing-library 驱动 编译后MarkdownDisplay,按渐进 token 前缀模拟串流(isPending=true),对比 BEFORE (#6340) 与 AFTER (#6345)
整应用(真实) 假的 OpenAI 串流服务 → 在 tmux 中跑真实 qwen TUI → 捕获表格逐行串流进来

真实 qwen TUI —— 串流一张 6 行表格(本 PR)

见上方第一张图。

  • t ≈ 2.5s —— 只显示句子;表头 不会| … | 纯文字逐字冒出来。
  • t ≈ 6.5s —— 整行一次性出现、一行接一行、是干净的方框(redraw-on-wider:Secondary 到达时 Role 栏加宽一次)。
  • t ≈ 12s —— 提交后的 6 行表格,横式格式一致(不翻转)。

Before (#6340) vs After (#6345) —— 相同 token 流

见上方第二张图。

  • 表头 hold-back ✅ —— BEFORE 把 | Name | Role | Team | 当纯文字漏出来;AFTER 在方框就绪前先藏住。
  • 整列原子化 ✅ —— BEFORE 把 | Alice | 渲染成 Role/Team 空格 的一行(并随每格到达重新加宽);AFTER 压住整行直到三栏齐全,再一步出现。
  • 栏宽 / 无逐 token reflow ✅、串流中格式稳定 ✅(串流中来一行很高的行也不会横→直翻转,锚定在第一行)。| grep foo(单 pipe)仍正常显示。

合并前建议修一处 —— 是描述,不是代码

描述里的 「No blank hang —— 表格一辨识出来就立刻画表头框(第一行还没到齐前),避免该区域空白」(以及 Evidence → After"a header box is shown immediately so the area never sits blank")已经和最终代码不一致了。commit 30f1610a「defer a streaming table until its first row (no empty-box flip)」)在 两个 RenderTable 输出点(MarkdownDisplay.tsx:502:718)都加了 tableRows.length > 0 守卫,所以 pending 期间零行表格根本不会被渲染。真实 TUI 也印证:t ≈ 2.5s 时表格区域是空白的(只有前置句子),直到 Alice 那一行凑满。

这其实是 更好 的行为 —— 干净的空白胜过「先画空框、再在长首行到达时翻成直式 label: value」—— 所以我建议 保留代码、改一下这段描述,例如:

「在第一行凑满前,该区域保持干净空白(没有纯 pipe 文字、也没有半填的行),从而避免空框 → 直式的翻转。」

补充:加了这个守卫后,TableRenderer.tsx 里的零行表头框路径 —— 包括早前 [Critical] review 在 :782 加的 safety-check 守卫 —— 从其唯一调用方(MarkdownDisplay目前已不可达。无害 / 防御性,但值得知道这条路径在这里并没有被走到。

次要说明(无需处理)

  1. 「串流与提交格式一致」 在串流过程中、以及均匀表格上都成立(已验证:串流格式 == 提交格式)。但一张 首行很短 / 后面某行很高 的极端表格,仍会在 提交那一刻 横→直翻转一次(串流锚定第一行 → 横式;提交量测所有行 → 直式)。calculateMaxRowLines 的注释已如实说明了这点 —— 只是提醒 Evidence 的措辞比实际行为略强。
  2. 多 pipe 的 非表格 行(| grep foo | wc -l,≥2 格)在串流中会被 暂时压住,直到后面出现一行非分隔线才释放 —— 已复现。是可接受的取舍(这类命令几乎都在代码块里);单 pipe 的 | grep foo 会立即显示。

结论: 串流表格的改进是真实的、在真实应用里验证过,相较 #6340 是明显进步;单测/lint/类型全绿。建议合并前把 「no blank hang / 立刻画表头框」 那条描述改成与最终 「defer until first row」 行为一致 —— 其余都是已注明或次要的点。


🤖 Generated with Claude Code — Claude Opus 4.8 (1M context)

@MikeWang0316tw

Copy link
Copy Markdown
Contributor Author

Thanks for the real-app Linux verification and the tmux before/after captures — that's exactly the behavior I was going for. 🙏

You're right that the description drifted from the shipped code. Fixed the PR body: the "No blank hang / immediate header box" bullet (and the Evidence → After line, plus the Chinese mirror) now describe the actual "defer until the first complete row" behavior — a clean blank (no raw pipe text, no half-filled rows) until the first row lands, deliberately avoiding the empty-box → vertical-label: value flip. Kept the code as-is, per your suggestion.

Good catch on the now-unreachable zero-row header-box path in TableRenderer.tsx (including the :782 safety guard from the earlier [Critical]): agreed it's dead from MarkdownDisplay's side after the tableRows.length > 0 guard. I've left it as defensive/documented rather than ripping it out in this PR, since it's the intended contract for that renderer if a future caller ever emits a zero-row table — happy to remove it in a follow-up if you'd prefer it gone.

The two minor notes (short-first/tall-later commit-time flip; multi-pipe non-table hold-back) are both already documented in code as accepted tradeoffs. Nothing else outstanding on my end.

@wenshao

wenshao commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: real and observed — streaming markdown tables show visible artifacts (cells trickling in one at a time, whole-table re-widening on a wider cell, header streaming in as raw pipe text). Builds on #6170 and #6340 which addressed related rendering issues. The problem is well-established.

Direction: aligned. TUI streaming polish is core to the product experience. No CHANGELOG reference needed for rendering fixes of this nature, but the area is clearly relevant.

Size: not applicable — all changes are in packages/cli/src/ui/utils/, outside core infrastructure paths.

Approach: scope feels right. Four tightly-coupled files (two source, two test), all focused on the same streaming table rendering problem. The five sub-improvements (atomic rows, width tracking, format stability, header hold-back, defer-until-first-row) each address a specific visible artifact and are well-motivated. No scope creep.

Moving on to code review and testing. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:真实存在——串流中的 markdown 表格有可见瑕疵(储存格逐格填入、更宽储存格让整张重画、表头以纯文字逐字冒出来)。建立在 #6170#6340 之上,问题已确认存在。

方向:对齐。TUI 串流体验打磨是产品核心。渲染修复不需要 CHANGELOG 参考,但该领域明确相关。

规模:不适用——所有改动在 packages/cli/src/ui/utils/,不在核心基础设施路径。

方案:范围合理。四个紧密耦合的文件(两个源码、两个测试),全部针对同一个串流表格渲染问题。五个子改进各自针对一个具体的可见瑕疵,动机明确,无范围蔓延。

进入代码审查与测试 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

The diff is well-structured and focused. Five sub-improvements in MarkdownDisplay.tsx (atomic rows, header hold-back, format stability, defer-until-first-row, code-fence/math-block tracking) each target a specific visible artifact. The separation of isPending (message streaming) from isFrontier (table at streaming edge) in TableRenderer.tsx is clean — the height clamp and format anchor track different concerns and the code comments explain why.

No critical blockers. The edge-case handling is thorough: code fences validate delimiter char and length, math blocks are tracked, non-table pipe lines (| grep foo) still render, mismatched column counts release rather than hold forever. The couldStillBeTable / hasMatchingSeparator asymmetry is deliberate and matches GFM semantics (separator must be the line immediately after the header).

Tests are comprehensive — 166 MarkdownDisplay + 71 TableRenderer, covering streaming vs committed format stability, frontier row hold-back, code-fence nesting, math-block interaction, and zero-row symmetry.

Unit Tests

237 passed (71 TableRenderer + 166 MarkdownDisplay)
tsc --noEmit: clean
eslint --max-warnings 0: clean

Real-Scenario Test (Interactive TUI)

PR code (npm run dev, branch fix/streaming-table-jitter HEAD f05d64c9) running in tmux on Linux. Prompted: "Output a 6-row 3-column markdown table: planet name, distance from sun in AU, number of moons"

TUI output (capture-pane, PR code)

  > Output a 6-row 3-column markdown table: planet name, distance from sun in
    AU, number of moons

  ∴ Thought for 0s (alt+t to expand)

  ◆
    ┌─────────┬────────────────────────┬───────┐
    │ Planet  │ Distance from Sun (AU) │ Moons │
    ├─────────┼────────────────────────┼───────┤
    │ Mercury │ 0.39                   │ 0     │
    ├─────────┼────────────────────────┼───────┤
    │ Venus   │ 0.72                   │ 0     │
    ├─────────┼────────────────────────┼───────┤
    │ Earth   │ 1.00                   │ 1     │
    ├─────────┼────────────────────────┼───────┤
    │ Mars    │ 1.52                   │ 2     │
    ├─────────┼────────────────────────┼───────┤
    │ Jupiter │ 5.20                   │ 95    │
    ├─────────┼────────────────────────┼───────┤
    │ Saturn  │ 9.58                   │ 146   │
    └─────────┴────────────────────────┴───────┘

The table renders as a clean horizontal box — proper box-drawing characters, correct column alignment, no raw pipe text, no empty header box. The model streamed fast enough that the completed table was captured in one snapshot, so I couldn't observe mid-stream cell-by-cell artifacts, but the final rendering is correct and matches the PR's claims.

Non-interactive comparison (installed qwen 0.19.1 vs PR code, -p mode)

Before (installed qwen -p):

| Language | Creator | Year |
|---|---|---|
| C | Dennis Ritchie | 1972 |
| Python | Guido van Rossum | 1991 |
| Java | James Gosling | 1995 |
| JavaScript | Brendan Eich | 1995 |
| Rust | Graydon Hoare | 2010 |
| Go | Robert Griesemer | 2009 |

After (PR code npm run dev -- -p):

| Planet  | Distance (AU) | Moons |
|---------|--------------:|------:|
| Mercury | 0.39          |     0 |
| Venus   | 0.72          |     0 |
| Earth   | 1.00          |     1 |
| Mars    | 1.52          |     2 |
| Jupiter | 5.20          |    95 |
| Saturn  | 9.58          |   146 |

Both produce correct output in -p mode (raw markdown, no TUI rendering). The streaming rendering improvements are only visible in the interactive TUI where the table appeared as a properly-formatted box.

Note: A collaborator (@wenshao) independently verified this PR with real tmux captures on Linux, including before/after screenshots showing the streaming improvements (header hold-back, atomic rows, width tracking, format stability). That verification confirms the streaming behavior claims.

中文说明

代码审查

Diff 结构清晰、聚焦。MarkdownDisplay.tsx 的五个子改进各自针对一个具体的可见瑕疵。TableRenderer.tsxisPending(消息串流)与 isFrontier(表格在串流前沿)的分离干净——高度钳制和格式锚定追踪不同关注点,代码注释解释了原因。

无关键阻塞。边界情况处理全面:代码栅栏验证分隔符字符与长度、数学块被追踪、非表格 pipe 行仍正常渲染、不匹配的栏数释放而非永久压住。couldStillBeTable / hasMatchingSeparator 的不对称是刻意的,符合 GFM 语义。

测试全面——166 个 MarkdownDisplay + 71 个 TableRenderer,涵盖串流与提交格式稳定性、前沿行压住、代码栅栏嵌套、数学块交互、零行对称。

单元测试

237 通过,tsc 干净,eslint 干净。

真实场景测试(互动式 TUI)

PR 代码在 Linux tmux 中运行。表格渲染为干净的横式方框——正确的 box-drawing 字符、正确的栏对齐、没有纯 pipe 文字、没有空表头框。协作者 @wenshao 也已独立在 Linux 上以真实 tmux 截图验证了串流改进。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Reflection

This PR is a polished, well-scoped follow-up to #6170 and #6340 that addresses the remaining visible artifacts in streaming table rendering. The five sub-improvements (atomic rows, width tracking, format stability, header hold-back, defer-until-first-row) each target a specific, observable jitter and the implementation matches what the PR promises.

The code is complex — the streaming hold-back logic has to track code fences, math blocks, header cell counts, separator column counts, and whether a table is at the frontier or closed by following text. But every piece of complexity is earned: each edge case (nested fences, $$ math, multi-pipe non-table lines, mismatched column counts) has a test and a comment explaining why it matters. The isPending/isFrontier separation is the right abstraction — two genuinely independent concerns (message streaming vs. table streaming) that were previously coupled.

The author has been responsive through multiple review cycles, fixing every finding including a critical code-fence tracking bug. A collaborator independently verified the streaming improvements on Linux with real tmux captures. 237 tests pass, tsc and eslint are clean.

One minor note: the description has been updated to match the shipped "defer until first row" behavior (per @wenshao's review). The now-unreachable zero-row header-box path in TableRenderer.tsx is dead code from MarkdownDisplay's perspective, but it's documented and harmless as defensive code.

Ships cleanly. Ready to merge. ✅

中文说明

总结

这个 PR 是 #6170#6340 的精心延续,解决了串流表格渲染剩余的可见瑕疵。五个子改进各自针对一个具体的、可观察的抖动,实现与 PR 所承诺的一致。

代码复杂——串流压住逻辑需要追踪代码栅栏、数学块、表头格数、分隔线栏数、以及表格是否在前沿或被后续行关闭。但每一分复杂度都值得:每个边界情况都有测试和注释解释其重要性。isPending/isFrontier 的分离是正确的抽象——两个真正独立的关注点此前耦合在一起。

作者在多轮审查中积极响应,修复了所有发现,包括一个关键的代码栅栏追踪 bug。协作者在 Linux 上以真实 tmux 截图独立验证了串流改进。237 个测试通过,tsc 和 eslint 干净。

可以合并。✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants